feat: link install scripts to relevant file in code tab#975
feat: link install scripts to relevant file in code tab#975danielroe merged 4 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughTwo new utility functions are introduced to 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| if (match?.[1]) { | ||
| // Script is `node <file-path>`, link to that file | ||
| // Normalize path: strip leading ./ | ||
| const filePath = match[1].replace(/^\.\//, '') | ||
|
|
||
| // Fall back to package.json if path contains navigational elements (the client-side routing can't handle these well) | ||
| if (filePath.includes('../') || filePath.includes('./')) { | ||
| return 'package.json' | ||
| } |
There was a problem hiding this comment.
Reject absolute paths to avoid broken code‑tab links.
Absolute paths (e.g., node /usr/bin/install.js) currently match and will be linked, even though they won’t exist in the package code. Treat absolute paths as non‑linkable and fall back to package.json/null.
🛠️ Suggested fix
- if (filePath.includes('../') || filePath.includes('./')) {
+ if (
+ filePath.startsWith('/') ||
+ filePath.includes('../') ||
+ filePath.includes('./')
+ ) {
return 'package.json'
}- if (filePath.includes('../') || filePath.includes('./')) {
+ if (
+ filePath.startsWith('/') ||
+ filePath.includes('../') ||
+ filePath.includes('./')
+ ) {
return null
}Also applies to: 169-172
| // Reconstruct the prefix (everything before the captured file path) | ||
| const prefix = scriptContent.slice(0, match.index + match[0].indexOf(match[1])) | ||
| return { prefix, filePath } |
There was a problem hiding this comment.
Prefix extraction breaks when the file name is node.
Using indexOf(match[1]) returns 0 for node node, producing an empty prefix and dropping the node part in the UI. Derive the prefix by length instead.
🛠️ Suggested fix
- const prefix = scriptContent.slice(0, match.index + match[0].indexOf(match[1]))
+ const prefix = scriptContent.slice(0, scriptContent.length - filePath.length)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Reconstruct the prefix (everything before the captured file path) | |
| const prefix = scriptContent.slice(0, match.index + match[0].indexOf(match[1])) | |
| return { prefix, filePath } | |
| // Reconstruct the prefix (everything before the captured file path) | |
| const prefix = scriptContent.slice(0, scriptContent.length - filePath.length) | |
| return { prefix, filePath } |
2 participants
if install script command is of the form ^node some-script-path$ it links to that script in the codetab, in all other cases it links to package.json